Abr tatyana #48
Abr tatyana #48
Conversation
| package edu.technopolis; | ||
| import java.io.Serializable; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Комментарии можно было не удалять. Или стоило написать свои, например, про особенности реализации.
| /* | ||
| * todo add complimentary fields if required | ||
| */ | ||
| private char[][] chunks; |
There was a problem hiding this comment.
Immutability или неизменяемость - один из важных паттернов проектирования классов. К тому же final имеет важную семантику с точки зрения многопоточного программирования. Так что final стоит поставить на все поля класса.
| * todo add complimentary fields if required | ||
| */ | ||
| private char[][] chunks; | ||
| int offset; |
There was a problem hiding this comment.
стоит писать комментарии с объяснением того, что такое offset. Иначе через месяц будет сложно разобраться, что имелось в виду.
| public CustomString(String str) { | ||
| this.count = str.length(); | ||
| this.length = str.length(); | ||
| this.chunkSize =(int)Math.sqrt(this.length)+1; |
There was a problem hiding this comment.
Эта стратегия выбора размера кусочка (chunk), на которые разбиваются строки очень спорная. Ну, или напишите комментарий, почему она имеет право на жизнь. Я привёл в пример конкретную задачу - поместить "Войну и мир" в такую строку с возможностью переиспользования.
По-моему, стоило сделать конструктор (String source, int chunkSize) с возможностью передачи размера чанка, и перегрузить конструктор (с такую сигнатурой, как у вас), вызывая из него this(source, DEFAULT_CHUNK_SIZE), передав в него какое-то большое значение по умолчанию, к примеру 10 000 символов.
| this.count = str.length(); | ||
| this.length = str.length(); | ||
| this.chunkSize =(int)Math.sqrt(this.length)+1; | ||
| this.chunks = new char[chunkSize][chunkSize]; |
There was a problem hiding this comment.
Кстати, в случае строки размером 1, вас размер чанка будет 2.
| /* | ||
| * todo add constructor or group of constructors | ||
| */ | ||
| public CustomString(char[][] chunks, int offset, int count, int chunckSize) { |
There was a problem hiding this comment.
Либо конструктор стоит сделать приватным, либо добавить большое кол-во проверок на совместимость аргументов, потому что неправильными настройками строки легко "прострелить себе ногу".
| public CharSequence subSequence(int start, int end) { | ||
| //todo implement subSequence here | ||
| if (start < 0 || start > end || end > length) { | ||
| throw new StringIndexOutOfBoundsException("Out of bounds"); |
There was a problem hiding this comment.
Непоследовательность. 10 строками выше выкидывается другое исключение.
| public char charAt(int index) { | ||
| //todo implement charAt here | ||
| if (index < 0 || index >= count) { | ||
| throw new IndexOutOfBoundsException("Out of bounds"); |
There was a problem hiding this comment.
В исключение стоит включить информацию о том, что собственно вышло за пределы массива. Например, писать - требовался такой-то символ, но длинна строки всего столько-то.
| if (start < 0 || start > end || end > length) { | ||
| throw new StringIndexOutOfBoundsException("Out of bounds"); | ||
| } | ||
| edu.technopolis.CustomString nc = new edu.technopolis.CustomString(chunks, offset + start, end - start + 1, chunkSize); |
There was a problem hiding this comment.
Здесь нет смысла использовать полное имя строки. Достаточно CustomString
| return outString.toString(); | ||
| } | ||
|
|
||
| public void printStr(edu.technopolis.CustomString str){ |
There was a problem hiding this comment.
метод либо статическим должен быть, либо печатать текщую строку.
| throw new StringIndexOutOfBoundsException("Out of bounds"); | ||
| } | ||
| edu.technopolis.CustomString nc = new edu.technopolis.CustomString(chunks, offset + start, end - start + 1, chunkSize); | ||
| return nc; |
There was a problem hiding this comment.
Здесь не совсем так реализован механизм переиспользования. Идея в том, чтобы использовать только те кусоки строки, которые нужны. Например, если вся "Война и мир" состоит из 1000 чанков, то в 4ом томе должно быть только 250 чанков. Иначе весь смысл разбиения на чанки теряется.
TimurTechnopolis
left a comment
TimurTechnopolis
left a comment
There was a problem hiding this comment.
Нужно исправить оставленные замечания. Больше комментариев в коде. Можно на русском языке.
TimurTechnopolis
assigned
TimurTechnopolis and
AbrTatyana and unassigned
TimurTechnopolis
2 participants
No description provided.